London |ITP-MAY-2025| Jamal Laqdiem| Book library #256
Conversation
jamallaqdiem
added
Needs Review
There was a problem hiding this comment.
- Can you check if any of this general feedback can help you further improve your code?
https://github.com/cjyuan/Module-Data-Flows/blob/book-library-feedback/debugging/book-library/feedback.md
Doing so can help me speed up the review process. Thanks.
- The Self-checklist in the PR description is not checked.
cjyuan
added
Reviewed
…js to handle and pass all criteria requested, answering questions in form of comments.
jamallaqdiem
added
the
Needs Review
debugging/book-library/script.js
Outdated
| return false; | ||
| } else { | ||
| let book = new Book(title.value,author.value, pages.value, check.checked); | ||
| let book = new Book(inputTitle.value,inputAuthor.value, pagesToNumber, check.checked); |
There was a problem hiding this comment.
On line 46, the raw title/author input could contain leading/trailing space characters, and pageToNumber can be a number with decimal places.
To reduce the chance of using raw input accidently, we could stored the pre-processed/sanitized/normalized input in some variables first, and reference the variables in other part of the function.
There was a problem hiding this comment.
Thanks for reviewing, I just stored the sanitized variables before suing them before creating the Book object.
| let readStatus = ""; | ||
| if (myLibrary[i].check == true) { | ||
| readStatus = "Yes"; | ||
| } else { | ||
| readStatus = "No"; | ||
| } | ||
| changeBut.innerText = readStatus; | ||
| changeReadStatusBtn.textContent = readStatus; |
There was a problem hiding this comment.
This could be a good chance to practice using the ? : conditional operator. We could rewrite the code on lines 86-92 as one statement.
There was a problem hiding this comment.
Thanks for reviewing, you're write much better and concise to do using ternary operator.
cjyuan
removed
the
Needs Review
jamallaqdiem
added
the
Needs Review
|
Changes look good! Well done. |
cjyuan
added
Complete
3 participants
Learners, PR Template
Self checklist
Changelist
Fixed the errors and the broken codes in the the script.js to make it work as intended.
Questions
Ask any questions you have for your reviewer.